feat(ui): unify activity feed alerts with notifications API (#25827) #27032
feat(ui): unify activity feed alerts with notifications API (#25827) #27032SaaiAravindhRaja wants to merge 3 commits intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
SaaiAravindhRaja
changed the title
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
There was a problem hiding this comment.
Pull request overview
Adjusts Notification Alerts pagination so page 1 no longer renders an extra (16th) row by accounting for the separately-fetched ActivityFeedAlert system alert.
Changes:
- Detects the “first page” fetch and reduces the API
limitby 1, then prependsActivityFeedAlertso the UI still showspageSizetotal rows. - Updates Playwright pagination test to re-enable standard row-count validation for Notification Alerts.
- Minor formatting change in
NotificationBoxinterface file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
openmetadata-ui/src/main/resources/ui/src/pages/NotificationListPage/NotificationListPage.tsx |
Changes first-page fetch behavior to avoid 16 rows by fetching pageSize - 1 and prepending ActivityFeedAlert. |
openmetadata-ui/src/main/resources/ui/src/components/NotificationBox/NotificationBox.interface.ts |
Adds a trailing newline (formatting only). |
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Pagination.spec.ts |
Removes special-casing so Notification Alerts pagination test validates row count again. |
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/pages/NotificationListPage/NotificationListPage.tsx:169
- The
isFirstPagecheck (afterandbeforeboth undefined) will be false when the user navigates back to page 1 via the "Previous" button, because that request uses abeforecursor. In that case page 1 will be rendered without theActivityFeedAlertrow and will fetchpageSizeitems (notpageSize - 1), making the first page content inconsistent depending on how it was reached.
Consider keying this logic off the page number (e.g., currentPage === 1 / the currentPage coming from the paging handler), and when navigating to page 1 clear the cursor params and fetch without cursor so the ActivityFeedAlert is always included on page 1.
const isFirstPage =
isUndefined(params?.after) && isUndefined(params?.before);
const { data, paging } = await getAllAlerts({
after: params?.after,
before: params?.before,
limit: isFirstPage ? pageSize - 1 : pageSize,
alertType: AlertType.Notification,
});
if (isFirstPage) {
// Fetch and show the system created activity feed alert on page 1
const activityFeedAlert = await getAlertsFromName(
'ActivityFeedAlert'
);
setAlerts([activityFeedAlert, ...data]);
} else {
setAlerts(data);
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ ApprovedUnifies activity feed alerts with the notifications API to standardize cross-platform delivery. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This issue has been closed, thank you for your submission! |
Fixes #25827
Unifies the Activity Feed Alerts fetch path with the paginated Notifications API, fixing the off-by-one bug where page 1 shows 16 items (15 notifications + 1 alert fetched separately).